-
Notifications
You must be signed in to change notification settings - Fork 14
CLOUDP-278403: Spectral README update for linting/OAS changes #257
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
tools/spectral/README.md
Outdated
1. Open a PR in MMS with the OAS changes and updating the MMS `.spectral.yaml` with the new/changed rules | ||
2. If the current `mongodb/openapi` spectral rules will violate the OAS changes, open a PR in `mongodb/openapi` and update/disable any rules that will fail | ||
3. Validate that the Spectral lint passes in `mongodb/openapi` and in MMS | ||
4. Review and merge both PRs | ||
5. Wait for the next release when the published OAS is updated | ||
6. Open a `mongodb/openapi` PR updating the linting `spectral-lint.yaml` with the linting changes initially added to MMS | ||
7. Validate that all tests pass | ||
8. Review and merge the PR | ||
9. Open a PR in MMS, updating the commit SHA of the imported spectral file, and removing any rules that were added to `mongodb/openapi` | ||
10. Validate all tests pass | ||
11. Review and merge the MMS PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure I understood these steps. In the scenario the new rule breaks the release, we need to update the MMS OAS (or the other services OASes depending on where is the error), backport the fix to the vbranch and wait 1 days that the service is deployed in all the env (dev, qa, staging and prod). once that happens, you can open a PR agains mongodb/openapi and update the spectral rule
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup that's pretty much it in a nutshell. It was a bit tricky to explain in clear steps, perhaps the contents right now are too verbose. Would you like me to try to make it a bit shorter and simpler? Or is there anything in the steps now that you wouldn't agree with?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @wtrocki on the fact that this is a public repo and we should not probably talk about MMS. I think you could add a link in the readme to the internal wiki: https://wiki.corp.mongodb.com/pages/viewpage.action?pageId=306447071 and move these steps there. You can even add the link to the PR template to make sure the developer had a look to the wiki before making changes to spectral.yml. Up to you if you want to add the wiki to the readme or to the template file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright sounds good, thanks!
tools/spectral/README.md
Outdated
1. Open a `mongodb/openapi` PR with the changes to `tools/spectral/.spectral.yaml` | ||
2. Validate that the new Spectral lint checks pass | ||
3. Review and merge the PR | ||
4. Open a PR in MMS, updating the commit SHA of the imported spectral file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI: For public repositories, we keep internal documentation in wikis. It allows us to put links and all information without being restricted in any way.
Fine to keep it here but feel free to move it to wiki if that helps your use case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, though I think the current content is alright to keep here (also suggested by @blva to update this README). @andreaangiolillo Are you good with keeping the contents here, or would you prefer an internal wiki linked through a golink instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Went with moving the contents to the wiki and keeping the README lighter for public contributions, RFAL!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks for all the changes!
Proposed changes
Updating the README for spectral changes to guide MMS/openapi maintainers depending on the change to the linting/OAS and changes needed to propagate to MMS.
_Jira ticket: CLOUDP-278403
Checklist